All committers have signed the CLA.

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 49.39%. Comparing base (7cc7ebb) to head (253d7a9).

@@            Coverage Diff             @@
##             main    #3445      +/-   ##
==========================================
+ Coverage   49.36%   49.39%   +0.03%     
==========================================
  Files         522      522              
  Lines       59465    59473       +8     
==========================================
+ Hits        29353    29375      +22     
+ Misses      26647    26639       -8     
+ Partials     3465     3459       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Looks good to me. There is just one thing we should probably take care of as per this comment: The appservice consumer should ignore m.room.redaction events if they come in through OutputTypeNewRoomEvent. See this. (Though I don't know if that really matters for bridges/appservices, not aware of how they are handled by them.)

OK great, I will look at the failing tests then and add one for this.

I am not sure I understand your comment (or rather, understand the spec...) because I thought OutputTypeRedactedEvent was a validated event that could not be part of a OutputTypeNewRoomEvent?

If we add a check for receivedType == api.OutputTypeNewRoomEvent || on the if statement on this line:

https://github.com/element-hq/dendrite/pull/3445/files#diff-594a94eeec27d0c54996b75cd3002ce09168b17432b108d759fb41c6a35e2a4fR151

does that resolve it?

I meant that we need this right before this line for this:

	// In order to honour redactions correctly, downstream components must ignore m.room.redaction events emitted via OutputTypeNewRoomEvent.
	// When downstream components receive an OutputTypeRedactedEvent they must:
	// - Pull out the event to redact from the database. They should have this because the redaction is validated.
	// - Redact the event and set the corresponding `unsigned` fields to indicate it as redacted.
	// - Replace the event in the database.

While we don't do anything with the DB in the appservice consumer, we simply forward the redacted event to the appservice/bridge.